server: consistent behaviour for list apis with project=-1#11767
server: consistent behaviour for list apis with project=-1#11767DaanHoogland merged 2 commits intoapache:mainfrom
Conversation
Fixes apache#9602 Fixes apache#11517 Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11767 +/- ##
============================================
+ Coverage 17.50% 17.56% +0.05%
- Complexity 15427 15521 +94
============================================
Files 5894 5906 +12
Lines 526845 528382 +1537
Branches 64334 64529 +195
============================================
+ Hits 92232 92791 +559
- Misses 424236 425160 +924
- Partials 10377 10431 +54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes consistent behavior for list APIs when project parameter is set to -1, ensuring that all networks (including project networks) are included in the search results when project=-1 is specified.
Key changes:
- Modified the
addProjectNetworksConditionToSearchmethod to handle the special case of projectId=-1 - Updated all callers to pass the projectId parameter consistently
- Added comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| NetworkServiceImpl.java | Enhanced project network filtering logic to handle project=-1 case and updated method signatures |
| NetworkServiceImplTest.java | Added unit tests to verify the new project network filtering behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@shwstppr, thanks for the PR! I believe after this one gets merged, we'll be able to move forward with the apache/cloudstack-cloudmonkey#185 PR. |
|
@blueorangutan package |
|
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Tested some of the APIs:
Only listNetworks was having different behaviour so fixed that. Testing results around that are added in the description. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15429 |
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14646)
|
5 participants
Description
Fixes #9602
Fixes #11517
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Created 3 new accounts:
Create a new Project
test-projusing root admin account and addeduseranddadmininto it. Create a test network for the project.Created networks using each of the four accounts -
admin, dadmin, user, d1adminThere are now 5 networks:
Verified when project toggle is changed:
admincan list all 5 networksdadmincan list all 5 networksusercan list 2 networks (user-net, testproj-net)d1admincan list only one network (d1admin-net)How did you try to break this feature and the system with this change?